Skip to content

fix(plugin-auth): 限流计数器惰性解析 kernel cache —— 误报的告警，与它掩盖的共享限流功能洞 - #4788

Merged
os-zhuang merged 1 commit into
mainfrom
claude/issue-4772-auth-lazy-cache-resolve
Aug 3, 2026
Merged

fix(plugin-auth): 限流计数器惰性解析 kernel cache —— 误报的告警，与它掩盖的共享限流功能洞#4788
os-zhuang merged 1 commit into
mainfrom
claude/issue-4772-auth-lazy-cache-resolve

Conversation

@os-zhuang

@os-zhuang os-zhuang commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #4772

先回答 PM 要求先验证的那个问题：是功能洞，不只是日志误报

结论：是。init() 时定下的「没有 cache」结论会被冻结整个进程生命周期，后续限流计数一直用的是 better-auth 的进程内 memory store，即使 cache 随后注册上也不会切换。建议给本 issue 补 security 标签。

证据链（全部来自代码，不是推测）：

  1. AuthPlugin.init() 里那次 getServiceAsync('cache') 探测的结果写进 authConfig.secondaryStorage，随后 new AuthManager(authConfig) 把它存进 this.config。
  2. AuthManager 的 better-auth 实例是懒创建的（getOrCreateAuth() → createAuthInstance()），但它读的是 init 时那份 config：...(this.config.secondaryStorage ? { storage: 'secondary-storage' } : {})。所以「懒创建」并没有让探测重来一次 —— 冻结的是结论，不是时机。
  3. init() 早于 CacheServicePlugin 注册 cache（issue 现场：早 21ms），所以标准 serve 组合下这个分支永远走 else。
  4. 于是 better-auth 落到默认 rateLimit.storage: 'memory' —— node_modules/better-auth/dist/api/rate-limiter/index.mjs 里那个模块级 memory Map，每个进程一份。

也就是说：多节点部署的限额从来没有被全局强制过，攻击者轮换节点即可把限额乘以节点数；而日志还在告诉运维「去接 Redis」，接完 Redis 仍然是同一条 warn、同一个洞。ADR-0069 D2 声明的能力与运行时不一致（Prime Directive #10 的形态）。

红→绿也验证过：把 auth-plugin.ts stash 掉、只留新测试，新增的 5 条插件级测试有 4 条失败。

改了什么

取 PM 裁定的修法 2（惰性解析），落点是 better-auth 的 rateLimit.customStorage。

新增 packages/plugins/plugin-auth/src/rate-limit-storage.ts：

  • createLazyCacheRateLimitStorage({ resolveCache, logger }) —— 计数器被消费时才去解析 cache 服务。这一刻必然在 kernel:ready 之后，因此与任何插件启动顺序无关；解析到之后句柄缓存复用。
  • 保留告警，但改了触发时机：只有当一个计数器真的要用共享存储、而此刻确实一个 cache 服务都没有时才打，每进程一次。那时它才是真信号。
  • 真没有 cache 的部署仍然限流，退化成进程内定宽窗口计数（降级，不是关闭）—— customStorage 会整体接管 better-auth 的存储选择，所以降级路径必须自己会数。
  • 计数算法（定宽窗口、不随请求滑动、只把剩余秒数交给存储）抽成 incrementFixedWindow，与 secondary-storage.ts 的 increment 共用一份实现，两个计数入口不可能漂移。

AuthManagerOptions 新增 rateLimitStorage（counters-only）。它刻意不放在 rateLimit 里面：bindAuthSettings 在管理员调限流参数时会整个替换 rateLimit 对象，放进去等于「一改设置就悄悄退回不共享」，有测试钉住。

一个必须让维护者知道的连带发现：为什么不用 secondaryStorage

issue 的直觉修法是「把 cache 真的接成 secondaryStorage」。不能这么修，否则会在修一个安全洞的同时开另一个：

better-auth 1.7.0-rc.2 的 internal-adapter.mjs：

  • createSession：if (secondaryStorage && !storeInDb) —— 设了 secondaryStorage 就不写 sys_session 行；
  • findSession：先读 secondaryStorage.get(token)，命中直接返回，完全不查库（即使打开 storeSessionInDatabase 双写，读路径依然以缓存为准）。

而 ADR-0069 D4 的三个会话管控（enforceSessionControls 的空闲/绝对超时、enforceConcurrentCap 的并发上限）全部靠写 sys_session 行来撤销会话，而且是 best-effort、异常吞掉。所以一旦 cache 被绑成 secondaryStorage：查不到行 → 直接 return，三个管控静默失效；就算双写，写进库的撤销 better-auth 也读不到，缓存快照最长活到会话 TTL（默认 7 天）。外加 sys_session 空表会牵连 sys_presence / sys_oauth_access_token / sys_oauth_refresh_token 三处 lookup 外键和会话列表 UI。

这个冲突一直没被发现，正是因为那次探测从来没成功过 —— 声明与运行时不一致把两个问题一起藏了起来。

本 PR 的处理：不替维护者决定会话该存哪。

验收对照

issue 验收标准 落点
1. 配了 cache 的部署不再出现这条 warn auth-plugin.test.ts → does not warn during init when the cache has not registered yet；有 cache 时改打一条 info
2. 真没配 cache 的部署仍然告警，两种情况可区分 rate-limit-storage.test.ts → warns exactly once, at counting time, and says what is actually wrong + stops warning once the cache is there（有 cache 全程零 warn）
3. 证实功能洞 → 加测试证明 cache 后注册时限流真的用上共享 cache rate-limit-storage.test.ts → picks the shared cache up on the first consume that follows registration；插件级同题 counts in the cache registered AFTER init
4. 改动限定在 packages/plugins/plugin-auth 是。另有 .changeset/ 与 docs/adr/0069（状态行事实订正），未碰 packages/objectql、service-storage、service-automation、plugin-approvals

测试

pnpm --filter @objectstack/plugin-auth test
 Test Files  29 passed (29)
      Tests  632 passed (632)

pnpm --filter @objectstack/plugin-auth typecheck
 (tsc --noEmit, no output)

红→绿证据（stash 掉 auth-plugin.ts 后跑新测试）：Tests 4 failed | 1 passed | 60 skipped。

另跑通：check:adr-anchors、check:durability-log-level、check:init-service-contract、check:service-providers。

🤖 Generated with Claude Code

https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny

Generated by Claude Code

@claude
fix(plugin-auth): resolve the kernel cache at counting time, not at i…
94c273b 
…nit (#4772)

`AuthPlugin.init()` probed `getServiceAsync('cache')` and froze the answer for
the life of the process. It runs BEFORE `CacheServicePlugin` registers the
service (21ms earlier in a showcase cold start), so the probe resolved
`undefined` in deployments that have a cache configured — and the warning it
printed told the operator to provision Redis for a problem they did not have.

The misdiagnosis was the visible half. The real defect: better-auth is built
lazily but from the config captured at init, so the "no cache" conclusion was
permanent. Rate-limit counters never reached the shared store even after it
came up, meaning a multi-node deployment's limits were never enforced globally
(ADR-0069 D2 declared a capability the runtime did not deliver).

`createLazyCacheRateLimitStorage()` implements better-auth's
`rateLimit.customStorage` and resolves the `cache` service when a counter is
actually consumed — strictly after `kernel:ready`, therefore independent of
plugin start order. The warning is kept but now fires only when a counter
genuinely has nowhere shared to count, once per process; without a cache the
limit is still enforced, in-process (degraded, never disabled).

Deliberately `customStorage`, not `secondaryStorage`: the latter also moves the
session of record into the cache (`createSession` skips the `sys_session` row,
`findSession` answers from the snapshot without reading the database), which
silently disables the ADR-0069 D4 session controls — idle timeout, absolute max
and concurrent cap all revoke by writing that row. The cache is therefore no
longer auto-bound as `secondaryStorage`; `cacheSecondaryStorage` is exported for
a host that opts into that trade knowingly. Where the session of record belongs
is #4785.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny
@vercel

vercel Bot commented Aug 3, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 3, 2026 6:30am

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling size/l and removed documentation Improvements or additions to documentation tests tooling labels Aug 3, 2026
@os-zhuang os-zhuang mentioned this pull request Aug 3, 2026
Closed
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth.

10 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx (via @objectstack/plugin-auth)
  • content/docs/deployment/production-readiness.mdx (via @objectstack/plugin-auth)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/plugin-auth)
  • content/docs/permissions/authentication.mdx (via @objectstack/plugin-auth)
  • content/docs/permissions/sso.mdx (via @objectstack/plugin-auth)
  • content/docs/plugins/index.mdx (via @objectstack/plugin-auth)
  • content/docs/plugins/packages.mdx (via @objectstack/plugin-auth)
  • content/docs/protocol/kernel/http-protocol.mdx (via @objectstack/plugin-auth)
  • content/docs/releases/implementation-status.mdx (via @objectstack/plugin-auth)
  • content/docs/releases/v9.mdx (via @objectstack/plugin-auth)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

This was referenced Aug 3, 2026
Open
Closed
@os-zhuang
os-zhuang marked this pull request as ready for review August 3, 2026 06:44
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 3, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 3, 2026
@os-zhuang
os-zhuang added this pull request to the merge queue Aug 3, 2026
@os-zhuang os-zhuang mentioned this pull request Aug 3, 2026
Open
Merged via the queue into main with commit f2eb850 Aug 3, 2026
21 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-4772-auth-lazy-cache-resolve branch August 3, 2026 07:12
This was referenced Aug 3, 2026
Closed
Merged
Merged
Open
akarma-synetal pushed a commit to akarma-synetal/framework that referenced this pull request Aug 3, 2026
@os-zhuang @claude
fix(plugin-auth): count the per-number OTP send budget in the shared …
8bd437f 
…store (objectstack-ai#4790) (objectstack-ai#4806)

objectstack-ai#2780's per-number OTP budget (60s cooldown + 5/hour) was shared across nodes
ONLY when a host supplied better-auth's `secondaryStorage`. Nothing in the
standard `serve` composition supplies one — and since objectstack-ai#4788, AuthPlugin
deliberately does not derive it from the kernel cache either — so the budget
was counted per process: an N-node deployment granted one phone number N
cooldowns and N hourly caps, in paid SMS, with no signal that the declared
limit was not the enforced one (ADR-0049).

Same defect class as objectstack-ai#4772's rate-limit counters, and now the same cure rather
than a second implementation of it. The lazy-resolution half of
`createLazyCacheRateLimitStorage` is extracted as `createLazyCounterStore()`:
resolve the `cache` service when a counter is CONSUMED (strictly after
`kernel:ready`, so plugin start order decides nothing), memoise the handle,
fall back to the bounded in-process store when there is genuinely no cache —
and say which of the two happened, once. The OTP guard reaches it through the
new `AuthManagerOptions.sharedCounterStore`, filled by AuthPlugin from the
same `resolveCache` closure the rate-limit counters use.

Deliberately NOT `secondaryStorage` (objectstack-ai#4785): that also relocates the session of
record into the cache and silently disables the ADR-0069 D4 session controls. A
host-supplied `secondaryStorage` still wins for this budget, unchanged.

The cooldown / rolling-hour semantics are untouched — only where the timestamps
live changed. A fixed-window counter cannot express "N seconds since the last
send", and converting the hourly cap to one would admit a 2× burst across the
window boundary: trading one multiplication for another.


Claude-Session: https://claude.ai/code/session_015Br2xsJsczFsTR9bvbh2Ny

Co-authored-by: Claude <noreply@anthropic.com>
@os-zhuang os-zhuang mentioned this pull request Aug 3, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

size/l

Projects

None yet

Milestone

No milestone

2 participants

@os-zhuang @claude